Deprecate `containerPackageInfo` parameter in `createOdspCreateContainerRequest` #23919

MarioJGMsoft · 2025-02-25T18:24:15Z

Description

As of PR #22849 (released in 2.23), preferred pattern to add "containerPackageName" parameter to request is via URL resolver instead of createOdspCreateContainerRequest's last parameter.
Tag calling createOdspCreateContainerRequest with that parameter as deprecated now, and then in the 2.40 release remove that form altogether.

Future Breaking Changes

See issue 23882 for more details.

Reviewer Guidance

Please let me know if there's anything else I should be aware of or can do better, thanks!

Fixes: AB#31450

…meter containerPackageInfo

MarioJGMsoft · 2025-02-25T18:33:35Z

examples/utils/webpack-fluid-loader/src/odspUrlResolver.ts

@@ -11,6 +11,7 @@ import {
 } from "@fluidframework/odsp-doclib-utils/internal";
 import {
 	OdspDriverUrlResolver,
+	// eslint-disable-next-line import/no-deprecated


Had to disable import no deprecated, because even though the version that is being imported is technically not the deprecated version, eslint detected that it was using that one. Does anyone know if there's a better way to deprecate the parameter?

The linter doesn't understand / is being conservative. This is okay, but should add a comment with the work item that will clean this up.

I think the only alternative might be to import the module as in import * as OdspDriver from "....
But since it is temporary, this is good enough.

Still looking for comment with the work item number.

Added comment

I recently learned of a better format to comment lint disables using --:

// eslint-disable-next-line import/no-deprecated -- This is the non-deprecated call, but eslint doesn't distinguish. TODO AB#31049 remove suppression

packages/drivers/odsp-driver/src/createOdspCreateContainerRequest.ts

Josmithr

Left a suggestion to enhance the deprecation notice comment. Otherwise, the API changes look good.

jason-ha · 2025-02-27T18:10:58Z

examples/utils/webpack-fluid-loader/src/odspUrlResolver.ts

+	// eslint-disable-next-line import/no-deprecated
 	createOdspCreateContainerRequest,
 	createOdspUrl,
 } from "@fluidframework/odsp-driver/internal";


This is an example and should not be importing from /internal. Recommend fixing these when we have to touch them. See examples that use examples/.eslintrc.cjs and/or examples/.eslintrc.data.cjs.

cc @Abe27342 and @alexvy86 -- We were just talking about these in Foundation Enhancements meeting. This example which does stuff with ODSP Driver probably should be in a different group of examples that are internal-only. We don't expect anyone in the public to be doing anything with ODSP Driver.

Should none of the odsp-driver exports be /legacy then? At least some of these are and should be okay to use /legacy import instead of /internal.

thread nudge

more aggressive nudge (@markfields , @Abe27342 , @alexvy86 )

packages/drivers/odsp-driver/src/createOdspCreateContainerRequest.ts

jason-ha · 2025-02-27T18:36:31Z

packages/test/test-drivers/src/odspDriverApi.ts

+	// eslint-disable-next-line import/no-deprecated
 	createOdspCreateContainerRequest,
 	createOdspUrl,
 } from "@fluidframework/odsp-driver/internal";


Other option for this and other /internal import is to make the /internal export only export the non-deprecated version.
The how-to is covered at https://github.com/microsoft/FluidFramework/wiki/Change-Recipes#moving-api-to-internal

The ./internal.ts version could look like:

export const odspCreateContainerRequest: ( siteUrl: string, driveId: string, filePath: string, fileName: string, createShareLinkType?: ISharingLinkKind, ) => ReturnType<typeof createOdspCreateContainerRequestOriginal> = createOdspCreateContainerRequestOriginal;

Since there are only two internal uses, may not be worth the effort.

I think it's not worth the effort.

jason-ha · 2025-02-27T18:52:11Z

I don't quite follow this statement in the description: "With PR: #22849 we introduced a parameter for ODSP that will not be used."
The parameter is used when passed. When present there will be a query parameter containerPackageName=....
Maybe you mean to say that there is alternative? Or are you saying that the generated QP will later be ignored?

MarioJGMsoft · 2025-02-27T19:09:44Z

I don't quite follow this statement in the description: "With PR: #22849 we introduced a parameter for ODSP that will not be used." The parameter is used when passed. When present there will be a query parameter containerPackageName=.... Maybe you mean to say that there is alternative? Or are you saying that the generated QP will later be ignored?

The query parameter (I'm guessing that's what QP stands for), will later be ignored. The new alternative is that it can be added in the constructor of OdspDriverUrlResolverForShareLink. Then it will be processed and added when it resolves a request.

Therefore, having the parameter in the request is unnecessary.

jason-ha · 2025-02-27T19:56:29Z

I don't quite follow this statement in the description: "With PR: #22849 we introduced a parameter for ODSP that will not be used." The parameter is used when passed. When present there will be a query parameter containerPackageName=.... Maybe you mean to say that there is alternative? Or are you saying that the generated QP will later be ignored?

The query parameter (I'm guessing that's what QP stands for), will later be ignored. The new alternative is that it can be added in the constructor of OdspDriverUrlResolverForShareLink. Then it will be processed and added when it resolves a request.

Therefore, having the parameter in the request is unnecessary.

I am not sure I understand what it exactly means to be processed and added when the request is resolved. Sounds like the deprecation issue needs some more content. If originally someone passed containerPackageInfo and got IRequest.url with containerPackageName=... that was then ..??.., what are all of the changes that are needed to get the same effect? Sounds like something should be changed in some resolver.?

ts-docs need corrected

packages/test/test-drivers/src/odspTestDriver.ts

packages/drivers/odsp-driver/src/createOdspCreateContainerRequest.ts

jason-ha · 2025-04-11T17:25:00Z

examples/utils/webpack-fluid-loader/src/odspUrlResolver.ts

@@ -11,6 +11,7 @@ import {
 } from "@fluidframework/odsp-doclib-utils/internal";
 import {
 	OdspDriverUrlResolver,
+	// eslint-disable-next-line import/no-deprecated


I recently learned of a better format to comment lint disables using --:

// eslint-disable-next-line import/no-deprecated -- This is the non-deprecated call, but eslint doesn't distinguish. TODO AB#31049 remove suppression

jzaffiro · 2025-04-11T21:11:01Z

.changeset/wicked-eagles-search.md

+"section": deprecation
+---
+
+The parameter `containerPackageInfo` in `createOdspCreateContainerRequest()` is deprecated and will be removed in version 2.40.


Suggested change

The parameter `containerPackageInfo` in `createOdspCreateContainerRequest()` is deprecated and will be removed in version 2.40.

Deprecate the `containerPackageInfo` parameter in `createOdspCreateContainerRequest()`

Applied changes

@tylerbutler and @jzaffiro - I think Mario got comments both ways on whether the removal version should be included in this text.

jzaffiro · 2025-04-11T21:12:06Z

.changeset/wicked-eagles-search.md

+
+The parameter `containerPackageInfo` in `createOdspCreateContainerRequest()` is deprecated and will be removed in version 2.40.
+
+This will mean that the name of the containerPackage can no longer be sent through the request. Instead it can be added in the constructor of `OdspDriverUrlResolverForShareLink`.


Suggested change

This will mean that the name of the containerPackage can no longer be sent through the request. Instead it can be added in the constructor of `OdspDriverUrlResolverForShareLink`.

The name of the containerPackage can no longer be sent through the request. This functionality will be removed in version 2.40. Instead, it can be added in the constructor of `OdspDriverUrlResolverForShareLink`.

Applied changes

.changeset/wicked-eagles-search.md

jzaffiro

Changeset looks good to me!

tylerbutler

Approving for docs.

github-actions · 2025-04-14T20:58:10Z

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output


> fluid-framework-docs-site@0.0.0 ci:check-links /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test "npm run serve -- --no-open" 3000 check-links

1: starting server using command "npm run serve -- --no-open"
and when url "[ 'http://127.0.0.1:3000' ]" is responding with HTTP status code 200
running tests using command "npm run check-links"


> fluid-framework-docs-site@0.0.0 serve
> docusaurus serve --no-open

[SUCCESS] Serving "build" directory at: http://localhost:3000/

> fluid-framework-docs-site@0.0.0 check-links
> linkcheck http://localhost:3000 --skip-file skipped-urls.txt

Crawling...

Stats:
  163664 links
    1315 destination URLs
    1547 URLs ignored
       0 warnings
       0 errors

…ontainerRequest` (#24481) ## Description When communicating with partners about work of [issue #23882](#23882), I realized that the code that was meant to be deleted will actually still be used, so there no longer is any need for the deprecation and removal. This PR is meant to be a revert for this one [PR #23919](https://github.com/microsoft/FluidFramework/pull/23919/files) ## Breaking Changes This will close [issue #23882](#23882) without actually deleting any code. ## Reviewer Guidance Let me know if you have any questions related to the revert or the code in general. Fixes: [AB#31049](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/31049)

refactor: created overload so that we can properly deprecate the para…

607654c

…meter containerPackageInfo

github-actions bot added area: driver Driver related issues area: odsp-driver public api change Changes to a public API base: main PRs targeted against main branch labels Feb 25, 2025

MarioJGMsoft linked an issue Feb 25, 2025 that may be closed by this pull request

[Won't Fix] Remove parameter in createOdspCreateContainerRequest() in v2.40 #23882

Closed

MarioJGMsoft mentioned this pull request Feb 25, 2025

Remove containerPackageInfo parameter in createOdspCreateContainerRequest #23872

Closed

refactor: had to disable import no deprecated.

ef5d850

github-actions bot added area: examples Changes that focus on our examples area: tests Tests to add, test infrastructure improvements, etc labels Feb 25, 2025

MarioJGMsoft commented Feb 25, 2025

View reviewed changes

Merge branch 'main' into deprecateRPI

ef2e8de

MarioJGMsoft marked this pull request as ready for review February 27, 2025 17:57

MarioJGMsoft requested a review from a team as a code owner February 27, 2025 17:57

MarioJGMsoft requested review from a team, pragya91, markfields, jason-ha, jatgarg, kian-thompson, WillieHabi and rajatch5 and removed request for a team February 27, 2025 17:57

Josmithr reviewed Feb 27, 2025

View reviewed changes

packages/drivers/odsp-driver/src/createOdspCreateContainerRequest.ts Outdated Show resolved Hide resolved

Josmithr previously approved these changes Feb 27, 2025

View reviewed changes

jason-ha reviewed Feb 27, 2025

View reviewed changes

docs: Disabled eslint deprecation

92234d7

github-actions bot added area: examples Changes that focus on our examples area: tests Tests to add, test infrastructure improvements, etc labels Mar 28, 2025

Merge branch 'main' into deprecateRPI

70ecfc4

jason-ha reviewed Mar 28, 2025

View reviewed changes

packages/test/test-drivers/src/odspTestDriver.ts Outdated Show resolved Hide resolved

packages/drivers/odsp-driver/src/createOdspCreateContainerRequest.ts Outdated Show resolved Hide resolved

packages/drivers/odsp-driver/src/createOdspCreateContainerRequest.ts Outdated Show resolved Hide resolved

MarioJGMsoft added 3 commits April 11, 2025 09:36

Merge branch 'main' into deprecateRPI

065add8

docs: applied changes based on comments

e9c3636

docs: applied changes based on comments

62c3774

jason-ha approved these changes Apr 11, 2025

View reviewed changes

pragya91 approved these changes Apr 11, 2025

View reviewed changes

jzaffiro reviewed Apr 11, 2025

View reviewed changes

jatgarg approved these changes Apr 11, 2025

View reviewed changes

docs: applied changes based on docs review

32f5cae

tylerbutler reviewed Apr 11, 2025

View reviewed changes

MarioJGMsoft added 2 commits April 11, 2025 23:34

docs: Applied changes to changeset based on comments

a0dcc11

docs: updated based on comments

65124c3

tylerbutler reviewed Apr 11, 2025

View reviewed changes

.changeset/wicked-eagles-search.md Outdated Show resolved Hide resolved

.changeset/wicked-eagles-search.md Outdated Show resolved Hide resolved

MarioJGMsoft added 2 commits April 14, 2025 16:24

applied changes based on comments.

3fd97f0

Applied changes based on comments

d981211

jzaffiro approved these changes Apr 14, 2025

View reviewed changes

Merge branch 'main' into deprecateRPI

cd8595a

tylerbutler added the release-blocking Must be addressed before we cut and publish the next release label Apr 14, 2025

tylerbutler approved these changes Apr 14, 2025

View reviewed changes

jatgarg and others added 2 commits April 14, 2025 13:45

Merge branch 'main' into deprecateRPI

6d645ea

Merge branch 'main' into deprecateRPI

d3ac29e

jatgarg merged commit 42b26b7 into microsoft:main Apr 14, 2025
36 checks passed

github-actions bot mentioned this pull request Apr 14, 2025

Upcoming Release: FluidFramework v2.43.0 #22101

Open

MarioJGMsoft deleted the deprecateRPI branch April 24, 2025 20:57

	The parameter `containerPackageInfo` in `createOdspCreateContainerRequest()` is deprecated and will be removed in version 2.40.
	Deprecate the `containerPackageInfo` parameter in `createOdspCreateContainerRequest()`


		The parameter `containerPackageInfo` in `createOdspCreateContainerRequest()` is deprecated and will be removed in version 2.40.

		This will mean that the name of the containerPackage can no longer be sent through the request. Instead it can be added in the constructor of `OdspDriverUrlResolverForShareLink`.

	This will mean that the name of the containerPackage can no longer be sent through the request. Instead it can be added in the constructor of `OdspDriverUrlResolverForShareLink`.
	The name of the containerPackage can no longer be sent through the request. This functionality will be removed in version 2.40. Instead, it can be added in the constructor of `OdspDriverUrlResolverForShareLink`.

Deprecate containerPackageInfo parameter in createOdspCreateContainerRequest #23919

Deprecate containerPackageInfo parameter in createOdspCreateContainerRequest #23919

Conversation

MarioJGMsoft commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Future Breaking Changes

Reviewer Guidance

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Josmithr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jason-ha commented Feb 27, 2025

Uh oh!

MarioJGMsoft commented Feb 27, 2025

Uh oh!

jason-ha commented Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jzaffiro left a comment

Choose a reason for hiding this comment

Uh oh!

tylerbutler left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 14, 2025

linkcheck output

Uh oh!

Uh oh!

Uh oh!

Deprecate `containerPackageInfo` parameter in `createOdspCreateContainerRequest` #23919

Deprecate `containerPackageInfo` parameter in `createOdspCreateContainerRequest` #23919

MarioJGMsoft commented Feb 25, 2025 •

edited

Loading

jason-ha commented Feb 27, 2025 •

edited

Loading